Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable rename plugin #2809

Merged
merged 21 commits into from
Apr 3, 2022
Merged

Conversation

OliverMadine
Copy link
Collaborator

@OliverMadine OliverMadine commented Mar 29, 2022

The rename plugin is disabled due to limited multi-component support in the HLS / hie-bios. This limitation causes renaming across components to work partially (depending which files have been indexed).


Within-module renaming

While the work on full multi-component support is continued, we can enable renaming of names within modules for use in both single- and multi-component projects.

I think that this is the better of the temporary solutions since it eliminates all partial behaviour, while still allowing the plugin to be of some use within multi-component projects.

The conditions required to perform a within-module rename are then:

  • the source of the name is the current module (not an imported name)
  • the current module must have an export list that does not contain the name

In the case that these are not met, we fail informatively.


Cross-module plugin option

This also adds plugin option for renaming across modules. This can be used in single-component projects or when ensuring that all components that reference the name are indexed.

By default cross-module renaming is disabled. It can be enabled with the following configuration option:

"haskell.plugin.rename.config.crossModule": true

Closes #2804
Closes haskell/vscode-haskell#560
Relevant issues: #2598

  - add context to error messages
  - remove unnecessary unwrapping of ParsedSource
  - use HashSet for references
  - consistent naming, whitespace, indentation, imports style
  - add rename package to hackage CI
  - set default build flag to True
@OliverMadine OliverMadine changed the title Rename within module Renaming within modules Mar 29, 2022
Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes no sense to me, it cripples the plugin artificially in an unrecoverable way.

Sigma IDE (a custom LSP server built with HLS) includes the rename plugin since the Sigma codebase is single component. Isn't there a way to preserve the full feature set in single component projects?

@OliverMadine
Copy link
Collaborator Author

OliverMadine commented Mar 29, 2022

This makes no sense to me, it cripples the plugin artificially in an unrecoverable way.

Sigma IDE (a custom LSP server built with HLS) includes the rename plugin since the Sigma codebase is single component. Isn't there a way to preserve the full feature set in single component projects?

We could preserve the full feature set in single-component projects but I don't think we have a way of determining if we are in a multi-component project or not?

Also as @michaelpj commented I think this would rule out most users?

If we could enable full support for single-component projects, then we could use this "within module" renaming just for multi-component projects. Maybe we could regex on the hie.yaml as a quick way of determining if the project is multi-component?

@pepeiborra
Copy link
Collaborator

We could preserve the full feature set in single-component projects but I don't think we have a way of determining if we are in a multi-component project or not?

I would suggest putting the behaviour in this PR behind a plugin option, enabled by default if that's what we want.

@michaelpj
Copy link
Collaborator

Pepe, when you say it cripples it "unrecoverably", do you mean specifically that you can't turn it off for Sigma?

I agree that making it possible to get the "full" plugin is a good idea, and that probably means a plugin option.

@michaelpj
Copy link
Collaborator

I think this is an okay step and provides at least some useful renaming to users (as opposed to nothing, currently). As and when we can support multiple components, we can just stop failing on non-local identifiers.

@pepeiborra
Copy link
Collaborator

Pepe, when you say it cripples it "unrecoverably", do you mean specifically that you can't turn it off for Sigma?

I mean that it's not possible to turn this change off, period. Anyone can do cabal install haskell-language-server with the rename flag to link this plugin and use it to do multi-component renames as long as they have indexed all the components.

@michaelpj
Copy link
Collaborator

I totally agree that we shouldn't actually lose the old functionality!

@OliverMadine OliverMadine changed the title Renaming within modules Enable rename plugin Apr 2, 2022
@OliverMadine
Copy link
Collaborator Author

Thanks for the feedback @pepeiborra @michaelpj! I’ve added a plugin option so that we can maintain the old functionality.

@pepeiborra
Copy link
Collaborator

I believe that an additional PR to https://github.com/haskell/vscode-haskell will be needed to expose the config option

@OliverMadine
Copy link
Collaborator Author

I believe that an additional PR to https://github.com/haskell/vscode-haskell will be needed to expose the config option

Done!

Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @OliverMadine for accepting all our suggestions and getting the rename plugin enabled at last!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable the rename plugin on single component projects Add support for using vscodes rename symbol feature
3 participants